Skip to content

Conversation

@Taris9047
Copy link

I've re-written readTemperature according to the datasheet. It seems the 16 bit temperature ADC stores bits to two separate 8 bit spaces. Converting them to signed int and again to float with temperature range, -40 to 85, then adjusting with + 25.0 seems to be yielding a close enough temperature.

Implemented float temperature reading.

Now LSM6SOXClass::readTemperature reads in float reference.
Changes according to last commit. readTemperature is now float type.
@CLAassistant
Copy link

CLAassistant commented Aug 7, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

github-actions bot commented Aug 7, 2022

Memory usage change @ 8b377d4

Board flash % RAM for global variables %
arduino:mbed_nano:nanorp2040connect 🔺 0 - +2260 0.0 - +0.01 0 - 0 0.0 - 0.0
Click for full report table
Board examples/SimpleAccelerometer
flash
% examples/SimpleAccelerometer
RAM for global variables
% examples/SimpleGyroscope
flash
% examples/SimpleGyroscope
RAM for global variables
% examples/SimpleTemperature
flash
% examples/SimpleTemperature
RAM for global variables
%
arduino:mbed_nano:nanorp2040connect 0 0.0 0 0.0 0 0.0 0 0.0 2260 0.01 0 0.0
Click for full report CSV
Board,examples/SimpleAccelerometer<br>flash,%,examples/SimpleAccelerometer<br>RAM for global variables,%,examples/SimpleGyroscope<br>flash,%,examples/SimpleGyroscope<br>RAM for global variables,%,examples/SimpleTemperature<br>flash,%,examples/SimpleTemperature<br>RAM for global variables,%
arduino:mbed_nano:nanorp2040connect,0,0.0,0,0.0,0,0.0,0,0.0,2260,0.01,0,0.0

@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Aug 8, 2022
Copy link
Contributor

@aentinger aentinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but this whole PR is a no-go.

You are changing an API on which others are already relying upon. Furthermore its done in the most complicated and invasive manner. This PR needs a complete rework with a separate function readTemperatureFloat, or I'll never approve it.

return 0;
}

temp_raw += ((int16_t)temp_H & 0b0000000011111111);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0x00FF is incredibly easier to read than 0b0000000011111111, please fix.

static int const TEMPERATURE_OFFSET_DEG = 25;

temperature_deg = (static_cast<int>(temperature_raw) / TEMPERATURE_LSB_per_DEG) + TEMPERATURE_OFFSET_DEG;
float fs = 125.0; /* -40 to 85 */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name fs does not feel like an improvement over TEMPERATURE_LSB_per_DEG. P

@aentinger
Copy link
Contributor

Implemented via #19 .

@aentinger aentinger closed this Aug 22, 2022
@per1234 per1234 added the conclusion: duplicate Has already been submitted label Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conclusion: duplicate Has already been submitted topic: code Related to content of the project itself type: enhancement Proposed improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants